-
Notifications
You must be signed in to change notification settings - Fork 14k
Add suggest alternatives for Out-of-range \x escapes #149201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The two-notes approach looks reasonable to me in the same error, but I really don't know anything about this part of the compiler so I'm going to reroll, sorry. |
fd040b7 to
02bc55a
Compare
|
I'm not 100% sure about note here, it feels to me that help would be better, because we are trying to help user to fix their incorrect code but I can be wrong about when to use each so correct me here |
02bc55a to
0f695f7
Compare
I'm neutral for |
| error: out of range hex escape | ||
| --> $DIR/out-of-range-hex-escape-suggestions-148917.rs:18:11 | ||
| | | ||
| LL | dbg!("\xFFFFF"); | ||
| | ^^^^ must be a character in the range [\x00-\x7f] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last one question and we are good to go: is this intended to have span like this for a string with len more than 4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, i think it's better to point to the span of full literal, fixed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like supporting strings is a bit painful, like for example
error: out of range hex escape
--> /home/gh-Kivooeo/test_/src/main.rs:2:11
|
2 | dbg!(" this is some kind of string \xa7");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must be a character in the range [\x00-\x7f]
error: aborting due to 1 previous errorwhich is not 100% correct, while I understand that calculating a precise span would be a challenging, would it be reasonable to remove string support for now and make it char only, it also will make implementation easier from what I see
upd: the current nightly already shows correct span
error: out of range hex escape
--> src/main.rs:2:40
|
2 | dbg!(" this is some kind of string \xa7");
| ^^^^ must be a character in the range [\x00-\x7f]There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh i see, we can't actually remove string support, I originally thought that we can because of this line, but it actually means not much matches!(mode, Mode::Char | Mode::Str)
so... idk, any ideas on this? Maybe we could somehow extend original err_span
0f695f7 to
9661d39
Compare
Fixes #148917
seems add two notes seems better.
r? @scottmcm